fix(logger): replace PassThrough with Writable for log handling COMPASS-9736 #7222
Conversation
There was a problem hiding this comment.
Pull Request Overview
Refactors the logger implementation to replace PassThrough stream with a custom Writable stream object for better control over log handling in the CompassWebLogger class.
- Replaces PassThrough stream with a custom Writable-like object that directly handles write and end operations
- Simplifies the logging pipeline by removing intermediate stream handling
- Maintains the same callback functionality for log processing
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| }); | ||
| const target = { | ||
| write(line: string, callback: () => void) { | ||
| callbackRef.current.onLog?.(JSON.parse(line)); |
There was a problem hiding this comment.
Do we want to update the test ensure it's happening synchronously?
nbbeeken
force-pushed
the
CLOUDP-332732
branch
from
33c13ae to
533f97a
Compare
packages/compass-connections/src/stores/connections-store-redux.ts
Outdated
Show resolved
Hide resolved
| log.info( | ||
| mongoLogId(1_001_000_005), | ||
| 'Compass Connection Attempt Started', | ||
| 'Connection attempt started', | ||
| { | ||
| clusterName: connectionInfo.atlasMetadata?.clusterName, | ||
| connectionId: connectionInfo.id, | ||
| } | ||
| ); |
There was a problem hiding this comment.
@syn-zhu On line 1583, there is log 1_001_000_004 that carries the same info, is there a significance to this logs position over that one?
There was a problem hiding this comment.
related to my response to your next comment: Our purpose for introducing these logs was to facilitate connection latency tracking with Sentry. Given that, I wanted to reduce the surface area to connection attempts that would actually be meaningful.
packages/compass-connections/src/stores/connections-store-redux.ts
Outdated
Show resolved
Hide resolved
|
There is one task failing, but it fails even on main: https://github.com/mongodb-js/compass/actions/runs/17210696051 Looks like it's been flaky for a while. Everything else is passing and I've addressed all comments, so gonna go ahead and merge this. |
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes